Skip to content

claircore: change Package.Kind type and add new values#1781

Open
hdonnay wants to merge 3 commits intoquay:mainfrom
hdonnay:feature/not-affected/package-kind
Open

claircore: change Package.Kind type and add new values#1781
hdonnay wants to merge 3 commits intoquay:mainfrom
hdonnay:feature/not-affected/package-kind

Conversation

@hdonnay
Copy link
Copy Markdown
Member

@hdonnay hdonnay commented Mar 19, 2026

This was largely an automated rewrite, but does touch a lot of files.

See-also: https://issues.redhat.com/browse/CLAIRDEV-85
Change-Id: I6ed030878dcccb1a5d0c437f2b480e9f0bea80fd
Signed-off-by: Hank Donnay hdonnay@redhat.com

@hdonnay hdonnay requested review from BradLugo and crozzy March 19, 2026 21:36
@hdonnay hdonnay requested a review from a team as a code owner March 19, 2026 21:36
@hdonnay hdonnay added this to the Not Affected milestone Mar 19, 2026
@hdonnay hdonnay force-pushed the feature/not-affected/package-kind branch 2 times, most recently from fe5217b to e8b5727 Compare March 26, 2026 15:37
Comment on lines +80 to +85
// An "ancestry" package is used to refer to a container layer and all
// previous layers in an image.
//
// Conceptually, an "ancestry" package is a higher type that describes the
// container image rather than the contents.
PackageAncestry // ancestry
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why, but this seems a bit unclear to me. Did you have some other ideas about this name? Or maybe we should explain why this value is important.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// An "ancestry" package is used to refer to a container layer and all
// previous layers in an image.
//
// Conceptually, an "ancestry" package is a higher type that describes the
// container image rather than the contents.
PackageAncestry // ancestry
// An "ancestry" package is used to refer to a container layer and all
// previous layers (a layer and its ancestors) in an image.
//
// Conceptually, an "ancestry" package is a higher type that describes the
// container image rather than the contents.
PackageAncestry // ancestry

@hdonnay hdonnay force-pushed the feature/not-affected/package-kind branch 2 times, most recently from 853067b to ccb5dd6 Compare March 27, 2026 20:38
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this fit with https://github.com/quay/claircore/blob/main/toolkit/types/package_kind.go? Is that replaced with this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably needs to be updated in turn. Will do.

Copy link
Copy Markdown
Contributor

@crozzy crozzy Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This order is now different: Unknown/Binary/Source vs Unknown/Source/Binary, although it seems like the toolkit one isn't actually used anywhere, should we just delete it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you mean. If you mean different to the claircore.PackageKind, then yes but they're different types.

I think I had this in here to facilitate moving the opposite way I did (like with the cpe package). I think this should stay if only for actual API reasons. I could also have this be the type instead of a new one in claircore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought was just that it's confusing having 2 types to represent what I think is the same thing, but I get that it's annoying to not have it next to the Package type. My preference would be to remove it from the toolkit, if we believe it's being used by someone that seems like we'd want to deprecate it and encourage movement to the new type (which has different Unmarshal behaviour IIUC). I could be missing something

@hdonnay hdonnay force-pushed the feature/not-affected/package-kind branch from ccb5dd6 to c5d4984 Compare March 30, 2026 18:25
This was largely an automated rewrite, but does touch a lot of files.

See-also: https://issues.redhat.com/browse/CLAIRDEV-85
Change-Id: I6ed030878dcccb1a5d0c437f2b480e9f0bea80fd
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
@hdonnay hdonnay force-pushed the feature/not-affected/package-kind branch from c5d4984 to 8a8d058 Compare March 30, 2026 20:18
hdonnay added 2 commits March 30, 2026 19:16
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Change-Id: Icbab38c7f062c8d4e057bd8ca89ab1c26a6a6964
JJ: See-Also: CLAIRDEV-NNNN
JJ: Closes: #NNNN
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Change-Id: Icbab38c7f062c8d4e057bd8ca89ab1c26a6a6964
@hdonnay hdonnay force-pushed the feature/not-affected/package-kind branch from e1b61f4 to 14447f5 Compare March 31, 2026 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants